Skip to content

server: Cleanup dhcp and dns entries only on expunging VM#3608

Merged
yadvr merged 1 commit intoapache:4.13from
shapeblue:vr-cleanup-vm-expunge
Sep 26, 2019
Merged

server: Cleanup dhcp and dns entries only on expunging VM#3608
yadvr merged 1 commit intoapache:4.13from
shapeblue:vr-cleanup-vm-expunge

Conversation

@yadvr
Copy link
Member

@yadvr yadvr commented Sep 25, 2019

This fixes a behaviour to not cleanup DHCP and DNS rules for NICs of a
VM in the VR when it is stopped, but instead when VM is expunged because
stopped VMs in CloudStack still retain the IPs and records.

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)

@yadvr
server: Cleanup dhcp and dns entries only on expunging VM
4eca1e4 
This fixes a behaviour to not cleanup DHCP and DNS rules for NICs of a
VM in the VR when it is stopped, but instead when VM is expunged because
stopped VMs in CloudStack still retain the IPs and records.

Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
@yadvr yadvr added this to the 4.13.1.0 milestone Sep 25, 2019
@yadvr
Copy link
Member Author

yadvr commented Sep 25, 2019

@blueorangutan package

@blueorangutan
Copy link

blueorangutan commented Sep 25, 2019

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan
Copy link

blueorangutan commented Sep 25, 2019

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-290

@yadvr
Copy link
Member Author

yadvr commented Sep 25, 2019

@blueorangutan test

@blueorangutan
Copy link

blueorangutan commented Sep 25, 2019

@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

anuragaw
anuragaw approved these changes Sep 26, 2019
View reviewed changes
Copy link
Contributor

@anuragaw anuragaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM from code.

shwstppr
shwstppr approved these changes Sep 26, 2019
View reviewed changes
Copy link
Contributor

@shwstppr shwstppr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM on changes!
Although dhcp cleanup on VM stop seems more logical to me.

@yadvr yadvr requested review from nvazquez and removed request for nvazquez September 26, 2019 11:33
nvazquez
nvazquez approved these changes Sep 26, 2019
View reviewed changes
Copy link
Contributor

@nvazquez nvazquez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, this makes cleanup introduced in this PR: #3351 to be performed only on VM expunge

wido
wido approved these changes Sep 26, 2019
View reviewed changes
Copy link
Contributor

@wido wido left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM based on code

@yadvr yadvr merged commit b853ebd into apache:4.13 Sep 26, 2019
@yadvr
Copy link
Member Author

yadvr commented Oct 2, 2019

FYI - this partially reverts and refactors changes introduced in #3351 to cleanup VM dhcp/dns rules in the VR only when a VM is expunged. A VM hostname and IP address is used as constraint and checks even of a VM is stopped, i.e. you cannot deploy a VM with the same hostname and IP in a network with an existing VM with those attributes. This was tested against a local monkeybox KVM env and regression tested agains Trillian/kvm.

ustcweizhou added a commit to ustcweizhou/cloudstack that referenced this pull request Oct 8, 2019
@ustcweizhou
server: Do NOT cleanup dhcp and dns when stop a vm
cecb713 
According comment in PR apache#3608, dhcp and dns entries are cleaned up only when a VM is expunged.
Revert part of commit 8fb388e.
@ustcweizhou ustcweizhou mentioned this pull request Oct 8, 2019
Merged
5 tasks
nvazquez added a commit to shapeblue/cloudstack that referenced this pull request Oct 10, 2019
@nvazquez
Backport apache#3608: server: Cleanup dhcp and dns entries only on ex…
cae3d23 
…punging VM
yadvr pushed a commit that referenced this pull request Oct 17, 2019
@ustcweizhou @yadvr
server: Do NOT cleanup dhcp and dns when stop a vm (#3627)
cc27095 
* server: Do NOT cleanup dhcp and dns when stop a vm

According comment in PR #3608, dhcp and dns entries are cleaned up only when a VM is expunged.
Revert part of commit 8fb388e.

* server: cleanup dns/dhcp entries in removeNic instead of finalizeExpunge
This was referenced Oct 29, 2019
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shwstppr shwstppr shwstppr approved these changes

@wido wido wido approved these changes

@nvazquez nvazquez nvazquez approved these changes

@borisstoyanov borisstoyanov Awaiting requested review from borisstoyanov

@GabrielBrascher GabrielBrascher Awaiting requested review from GabrielBrascher

+1 more reviewer

@anuragaw anuragaw anuragaw approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

component:compute component:virtual-router

Projects

None yet

Milestone

4.13.1.0

Development

Successfully merging this pull request may close these issues.

6 participants

@yadvr @blueorangutan @shwstppr @wido @nvazquez @anuragaw